Skip to content

feat: expose Claude Agent SDK options in session creation UI#1326

Merged
jeremyeder merged 16 commits intoambient-code:mainfrom
jeremyeder:010-advanced-sdk-options
Apr 16, 2026
Merged

feat: expose Claude Agent SDK options in session creation UI#1326
jeremyeder merged 16 commits intoambient-code:mainfrom
jeremyeder:010-advanced-sdk-options

Conversation

@jeremyeder
Copy link
Copy Markdown
Contributor

@jeremyeder jeremyeder commented Apr 16, 2026

Summary

  • Add collapsible "Advanced SDK Options" section to session creation, gated behind advanced-sdk-options workspace feature flag (disabled by default)
  • Backend allowlist (22 keys) + type validation filters sdkOptions on create request, serializes into SDK_OPTIONS env var on the CR
  • Runner parses SDK_OPTIONS with defense-in-depth denylist (10 keys), appends user system prompt under ## Custom Instructions heading
  • Frontend wraps existing claude-agent-options/ form components in a collapsible container
  • SDK options drift detection integrated into sdk-version-bump.yml workflow — detects field changes when SDK version bumps
  • Deletes broken daily-sdk-update.yml (targeted nonexistent claude-code-runner directory)
  • Fixes command injection in sdk-version-bump.yml (moved ${{ }} expressions to env: blocks)

Data Flow

Frontend form → sdkOptions on POST request
  → Backend: allowlist filter + type validation → JSON string
    → CR environmentVariables["SDK_OPTIONS"]
      → Runner: parse, denylist filter, merge into adapter options
        → ClaudeAgentAdapter(options)

Security

  • Backend allowlist: only permitted keys with valid types pass (22 allowed, 12 excluded)
  • Runner denylist: blocks cwd, api_key, mcp_servers, setting_sources, stderr, resume, continue_conversation, add_dirs, cli_path, env
  • System prompt: append-only under ## Custom Instructions heading
  • Feature flag: UI-only gate, API always accepts sdkOptions

Test plan

  • Backend: 25 Ginkgo specs (allowlist, type validation, handler wiring)
  • Runner: 32 pytest tests (parsing, denylist, system prompt merge, adapter integration)
  • Frontend: 5 vitest tests (flag gate, collapse, expand) + 636 total suite passing
  • Frontend build: 0 errors, 0 warnings
  • No any types in changed files
  • actionlint clean on all workflow files
  • CodeRabbit CLI: 0 findings (4 rounds, all addressed)
  • Drift detection: exits 0 locally (no drift with current manifest)

Supersedes #1146.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Advanced SDK Options UI in session creation (workspace feature flag).
    • Session creation accepts user-provided sdkOptions, validated and forwarded to the runtime; system prompt appends user instructions.
  • Chores

    • Removed legacy daily SDK-update workflow; added manifest-driven SDK drift check and integration into CI.
  • Tests

    • Added unit and integration tests for SDK options parsing, validation, merging, and frontend form behavior.
  • Documentation

    • Added spec, plan, tasks, and generated SDK-options manifest.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 16, 2026

Deploy Preview for cheerful-kitten-f556a0 canceled.

Name Link
🔨 Latest commit a19f253
🔍 Latest deploy log https://app.netlify.com/projects/cheerful-kitten-f556a0/deploys/69e14f31c5ab890008f492e0

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds end-to-end support for user-provided SDK options: frontend UI and types for sdkOptions, backend allowlist/type validation and serialization into SDK_OPTIONS, runner parsing with denylist and system_prompt merge, canonical SDK manifest plus drift-check script/tests, and workflow updates (removed legacy daily workflow).

Changes

Cohort / File(s) Summary
Workflow Automation
\.github/workflows/sdk-version-bump.yml, \.github/workflows/daily-sdk-update.yml
Reworked SDK bump workflow (moved PACKAGE_ARG to env, added SDK options drift-check step, changed PR/env propagation and summary logic) and removed the legacy daily workflow file.
API Types & Backend
components/backend/types/session.go, components/backend/handlers/sessions.go, components/backend/handlers/sessions_sdk_options_test.go
Added SdkOptions to CreateAgenticSessionRequest; implemented filterSdkOptions and validateSdkOptionValue (allowlist + per-key type checks); handler marshals filtered options into SDK_OPTIONS and returns 400 on validation/marshal errors; tests added.
Frontend Pages & Mutations
components/frontend/src/app/projects/[name]/new/page.tsx, components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/new-session-view.tsx, components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/__tests__/new-session-view.test.tsx
Session creation accepts optional sdkOptions; new-session-view adds react-hook-form/zod integration, computes non-default values for payload, and passes sdkOptions to create-session; tests mock advanced options/flags.
Create Session Dialog & Types
components/frontend/src/components/create-session-dialog.tsx, components/frontend/src/types/agentic-session.ts, components/frontend/src/types/api/sessions.ts
Renamed request field agentOptionssdkOptions; dialog now sources SDK options when the advanced-sdk-options flag is enabled.
Advanced SDK Options Component & Tests
components/frontend/src/components/advanced-sdk-options.tsx, components/frontend/src/components/__tests__/advanced-sdk-options.test.tsx
New AdvancedSdkOptions React component (form, Save/Cancel, unsaved confirmation) with unit tests; component accepts a react-hook-form instance and optional disabled/onSave.
Runner Bridge & Tests
components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py, components/runners/ambient-runner/tests/test_sdk_options.py
Added _parse_sdk_options and denylist handling for SDK_OPTIONS: parse JSON, drop denylisted keys, remove nulls, append system_prompt under “## Custom Instructions”, and merge into adapter options; tests validate parsing, denylist, logs, and merging.
SDK Manifest & Drift Script
components/runners/ambient-runner/sdk-options-manifest.json, scripts/sdk-options-drift-check.py
Added canonical sdk-options-manifest.json describing ClaudeAgentOptions fields and scripts/sdk-options-drift-check.py to introspect installed SDK, detect/update manifest drift, and exit with structured codes.
Feature Flag & Specs
components/manifests/base/core/flags.json, specs/010-advanced-sdk-options/...
Added workspace-scoped advanced-sdk-options flag and authored plan/spec/tasks documents describing end-to-end design, validation, system-prompt semantics, tests, and CI drift automation.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant FE as Frontend
    participant BE as Backend
    participant Runner as Ambient Runner
    participant SDK as Claude SDK

    User->>FE: Open Advanced SDK Options and submit values
    FE->>FE: validate/filter form (remove defaults/empties)
    FE->>BE: POST /sessions { sdkOptions }
    BE->>BE: filterSdkOptions (allowlist + type checks)
    alt validation error
        BE-->>FE: 400 Validation Error
    else valid
        BE->>BE: json.Marshal -> set SDK_OPTIONS env var
        BE-->>FE: 201 Session Created
    end
    Runner->>Runner: read SDK_OPTIONS env var
    Runner->>Runner: _parse_sdk_options (JSON, drop denylist, remove nulls)
    Runner->>Runner: merge system_prompt under "## Custom Instructions"
    Runner->>SDK: Instantiate ClaudeAgentAdapter with merged options
    SDK-->>Runner: Adapter initialized
Loading
🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title follows Conventional Commits format (feat: prefix) and accurately describes the main changeset: exposing SDK options in session creation UI.
Performance And Algorithmic Complexity ✅ Passed PR introduces no meaningful performance regressions. Backend handlers use O(1) map lookups, Python runner performs single-pass parsing, frontend JSON operations only on user interactions with small objects.
Security And Secret Handling ✅ Passed PR implements robust security controls: backend enforces allowlist of 22 permitted SDK options with strict type validation, authentication via GetK8sClientsForRequest(), runner applies denylist of 10 platform-internal keys with per-key warnings, safe JSON parsing without eval/pickle/exec risks, user system prompts appended under separate section, GitHub tokens moved to step-level env blocks, no hardcoded secrets, no sensitive data logging.
Kubernetes Resource Safety ✅ Passed No K8s manifests, RBAC policies, or resource definitions modified. Changes limited to backend handlers, frontend components, Python code, workflows, and configuration files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
components/backend/handlers/sessions.go (1)

896-904: ⚠️ Potential issue | 🔴 Critical

SDK_OPTIONS filtering is bypassable through raw env passthrough.

Line 930 filters req.SdkOptions, but Lines 902-904 already copy user-supplied environmentVariables into envVars. A direct API client can set environmentVariables.SDK_OPTIONS and skip the allowlist/type checks entirely whenever req.SdkOptions is omitted.

Suggested fix
  for k, v := range req.EnvironmentVariables {
+    if k == "SDK_OPTIONS" {
+      c.JSON(http.StatusBadRequest, gin.H{"error": "environmentVariables.SDK_OPTIONS is reserved; use sdkOptions"})
+      return
+    }
     envVars[k] = v
  }

Also applies to: 928-943

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 896 - 904, The merge of
user-supplied environment variables into envVars allows bypassing the SdkOptions
allowlist by setting "SDK_OPTIONS" in req.EnvironmentVariables; prevent this by
removing or rejecting the "SDK_OPTIONS" key from req.EnvironmentVariables before
merging (or explicitly ignore it when building envVars) and ensure only
req.SdkOptions (after the existing filtering logic around req.SdkOptions) can
influence SDK options; apply the same stripping/validation for the other similar
merge locations (the block around req.EnvironmentVariables and the block
covering lines ~928-943) so "SDK_OPTIONS" cannot be injected via raw env
passthrough.
.github/workflows/sdk-version-bump.yml (1)

35-46: ⚠️ Potential issue | 🟡 Minor

Pin GitHub Actions to commit SHAs.

Actions are using version tags (v6, v5) instead of SHA hashes, which is a supply-chain security risk.

Example SHA pinning
      - name: Checkout repository
-       uses: actions/checkout@v6
+       uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
        with:
          ref: main
          token: ${{ secrets.GITHUB_TOKEN }}

      - name: Setup Python
-       uses: actions/setup-python@v6
+       uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
        with:
          python-version: '3.11'

      - name: Install uv
-       uses: astral-sh/setup-uv@v5
+       uses: astral-sh/setup-uv@6b9c6063abd6010835644d4c2e1bef4cf5cd0fca # v5.3.1

As per coding guidelines, "Pin action versions to SHA."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/sdk-version-bump.yml around lines 35 - 46, Replace the
action version tags with pinned commit SHAs to mitigate supply-chain risks:
update the uses entries for actions/checkout@v6, actions/setup-python@v6, and
astral-sh/setup-uv@v5 to their corresponding full commit SHAs (e.g.,
actions/checkout@<commit-sha>), by looking up the exact repository tags/releases
and copying the commit SHA for the desired tag; ensure each uses line references
the full 40-character SHA and commit messages/PR are updated to note the
pinning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/backend/handlers/sessions.go`:
- Around line 68-204: The allowlist defined by allowedSdkOptionKeys is missing
18 manifest-defined SDK option keys, causing valid options to be dropped in
filterSdkOptions; update the allowedSdkOptionKeys map to include the keys listed
in the comment (model, fallback_model, continue_conversation, session_id,
settings, add_dirs, can_use_tool, cli_path, cwd, debug_stderr, fork_session,
max_thinking_tokens, mcp_servers, permission_prompt_tool_name, resume,
setting_sources, stderr, task_budget) and then add each key to the appropriate
type maps (sdkOptionStringKeys, sdkOptionNumericKeys, sdkOptionBoolKeys,
sdkOptionSliceKeys or leave as complex/object accepted by
validateSdkOptionValue) so validateSdkOptionValue enforces the correct types for
those new keys used by filterSdkOptions and session creation.

In `@components/frontend/src/components/create-session-dialog.tsx`:
- Around line 226-227: The sdkOptions assignment is only executed when
advancedAgentOptions is true, but the workspace-scoped flag is
"advanced-sdk-options" and is checked elsewhere with useWorkspaceFlag, so
request.sdkOptions never gets sent when the workspace flag is enabled; update
the condition around request.sdkOptions = agentOptionsForm.getValues()
(currently guarded by advancedAgentOptions) to also check the workspace flag
used by the advanced SDK component (e.g., call useWorkspaceFlag(projectName,
"advanced-sdk-options") or use the existing workspaceFlag variable) so that the
branch runs when either advancedAgentOptions or the workspace
"advanced-sdk-options" flag is enabled, ensuring sdkOptions is posted during
session creation.

In `@specs/010-advanced-sdk-options/spec.md`:
- Around line 94-95: FR-005's denylist enumeration is incomplete: update the
FR-005 requirement to include the missing keys `cli_path` and `env` alongside
the existing list (`cwd`, `api_key`, `mcp_servers`, `setting_sources`, `stderr`,
`resume`, `continue_conversation`, `add_dirs`) so the spec matches the
implemented security behavior; ensure the text for FR-005 still states that the
runner applies this denylist and logs a warning per blocked key, and leave
FR-006 unchanged.

In `@specs/010-advanced-sdk-options/tasks.md`:
- Around line 65-66: Remove or update the stale tasks T042 and T043: do not add
a new workflow file `.github/workflows/claude-sdk-options-drift.yml`; instead
consolidate drift detection into the existing `sdk-version-bump.yml` workflow
(or remove the checklist entries), and update any references to the drift script
`scripts/sdk-options-drift-check.py` to point at the consolidated job that runs
weekly (cron `0 6 * * 1`) and supports `workflow_dispatch`, implements the
exit-code handling (exit 1 -> create branch/commit/PR with `amber:auto-fix`,
exit 2 -> fail loudly), so the checklist no longer proposes creating a duplicate
workflow.

---

Outside diff comments:
In @.github/workflows/sdk-version-bump.yml:
- Around line 35-46: Replace the action version tags with pinned commit SHAs to
mitigate supply-chain risks: update the uses entries for actions/checkout@v6,
actions/setup-python@v6, and astral-sh/setup-uv@v5 to their corresponding full
commit SHAs (e.g., actions/checkout@<commit-sha>), by looking up the exact
repository tags/releases and copying the commit SHA for the desired tag; ensure
each uses line references the full 40-character SHA and commit messages/PR are
updated to note the pinning.

In `@components/backend/handlers/sessions.go`:
- Around line 896-904: The merge of user-supplied environment variables into
envVars allows bypassing the SdkOptions allowlist by setting "SDK_OPTIONS" in
req.EnvironmentVariables; prevent this by removing or rejecting the
"SDK_OPTIONS" key from req.EnvironmentVariables before merging (or explicitly
ignore it when building envVars) and ensure only req.SdkOptions (after the
existing filtering logic around req.SdkOptions) can influence SDK options; apply
the same stripping/validation for the other similar merge locations (the block
around req.EnvironmentVariables and the block covering lines ~928-943) so
"SDK_OPTIONS" cannot be injected via raw env passthrough.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4429a850-06c9-4976-80b7-d458be1b54ff

📥 Commits

Reviewing files that changed from the base of the PR and between 03f07d7 and 285140c.

📒 Files selected for processing (21)
  • .github/workflows/daily-sdk-update.yml
  • .github/workflows/sdk-version-bump.yml
  • components/backend/handlers/sessions.go
  • components/backend/handlers/sessions_sdk_options_test.go
  • components/backend/types/session.go
  • components/frontend/src/app/projects/[name]/new/page.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/__tests__/new-session-view.test.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/new-session-view.tsx
  • components/frontend/src/components/__tests__/advanced-sdk-options.test.tsx
  • components/frontend/src/components/advanced-sdk-options.tsx
  • components/frontend/src/components/create-session-dialog.tsx
  • components/frontend/src/types/agentic-session.ts
  • components/frontend/src/types/api/sessions.ts
  • components/manifests/base/core/flags.json
  • components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py
  • components/runners/ambient-runner/sdk-options-manifest.json
  • components/runners/ambient-runner/tests/test_sdk_options.py
  • scripts/sdk-options-drift-check.py
  • specs/010-advanced-sdk-options/plan.md
  • specs/010-advanced-sdk-options/spec.md
  • specs/010-advanced-sdk-options/tasks.md
💤 Files with no reviewable changes (1)
  • .github/workflows/daily-sdk-update.yml

Comment on lines +68 to +204
// allowedSdkOptionKeys defines the SDK options that users are allowed to configure.
// Keys NOT in this map are silently dropped by filterSdkOptions.
// Platform-internal keys (cwd, resume, mcp_servers, api_key, etc.) are excluded
// because they are set by the runner, not users.
var allowedSdkOptionKeys = map[string]bool{
// String keys
"system_prompt": true,
"permission_mode": true,
"effort": true,
"user": true,
// Numeric keys
"max_turns": true,
"max_budget_usd": true,
"max_buffer_size": true,
// Bool keys
"include_partial_messages": true,
"enable_file_checkpointing": true,
// Slice keys
"allowed_tools": true,
"disallowed_tools": true,
"betas": true,
"plugins": true,
// Complex object keys (maps, nested structures)
"thinking": true,
"tools": true,
"sandbox": true,
"output_format": true,
"hooks": true,
"agents": true,
"env": true,
"extra_args": true,
}

// sdkOptionStringKeys are SDK options that must be string values.
var sdkOptionStringKeys = map[string]bool{
"permission_mode": true,
"effort": true,
"user": true,
}

// sdkOptionNumericKeys are SDK options that must be numeric (float64 or int).
var sdkOptionNumericKeys = map[string]bool{
"max_turns": true,
"max_budget_usd": true,
"max_buffer_size": true,
}

// sdkOptionBoolKeys are SDK options that must be boolean.
var sdkOptionBoolKeys = map[string]bool{
"include_partial_messages": true,
"enable_file_checkpointing": true,
}

// sdkOptionSliceKeys are SDK options that must be slices ([]interface{}).
var sdkOptionSliceKeys = map[string]bool{
"allowed_tools": true,
"disallowed_tools": true,
"betas": true,
"plugins": true,
}

// validateSdkOptionValue performs basic type checking on a single SDK option value.
// nil values always pass. Complex object keys (thinking, sandbox, etc.) accept any value.
func validateSdkOptionValue(key string, value interface{}) error {
if value == nil {
return nil
}

// system_prompt can be string or map (preset format)
if key == "system_prompt" {
switch value.(type) {
case string, map[string]interface{}:
return nil
default:
return fmt.Errorf("sdkOptions.%s must be a string or object, got %T", key, value)
}
}

if sdkOptionStringKeys[key] {
if _, ok := value.(string); !ok {
return fmt.Errorf("sdkOptions.%s must be a string, got %T", key, value)
}
return nil
}

if sdkOptionNumericKeys[key] {
switch value.(type) {
case float64, int:
return nil
default:
return fmt.Errorf("sdkOptions.%s must be a number, got %T", key, value)
}
}

if sdkOptionBoolKeys[key] {
if _, ok := value.(bool); !ok {
return fmt.Errorf("sdkOptions.%s must be a boolean, got %T", key, value)
}
return nil
}

if sdkOptionSliceKeys[key] {
if _, ok := value.([]interface{}); !ok {
return fmt.Errorf("sdkOptions.%s must be an array, got %T", key, value)
}
return nil
}

// Complex object keys (thinking, sandbox, output_format, hooks, agents, env, extra_args, tools)
// accept any value — JSON serialization handles them.
return nil
}

// filterSdkOptions filters an SDK options map, keeping only allowed keys and
// validating primitive types. Unknown keys are silently dropped. Returns nil
// if the input is nil/empty or all keys were filtered out.
func filterSdkOptions(opts map[string]interface{}) (map[string]interface{}, error) {
if len(opts) == 0 {
return nil, nil
}

filtered := make(map[string]interface{})
for key, value := range opts {
if !allowedSdkOptionKeys[key] {
continue
}
if err := validateSdkOptionValue(key, value); err != nil {
return nil, err
}
filtered[key] = value
}

if len(filtered) == 0 {
return nil, nil
}
return filtered, nil
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python - <<'PY'
import json
import pathlib
import re

go_src = pathlib.Path("components/backend/handlers/sessions.go").read_text()
manifest = json.loads(pathlib.Path("components/runners/ambient-runner/sdk-options-manifest.json").read_text())

m = re.search(r'var allowedSdkOptionKeys = map\[string\]bool\{(.*?)\n\}', go_src, re.S)
if not m:
    raise SystemExit("allowedSdkOptionKeys not found in components/backend/handlers/sessions.go")

backend_keys = set(re.findall(r'"([^"]+)"\s*:\s*true', m.group(1)))
manifest_keys = set(manifest["options"].keys())

print("Only in backend allowlist:")
for key in sorted(backend_keys - manifest_keys):
    print("  -", key)

print("\nMissing from backend allowlist:")
for key in sorted(manifest_keys - backend_keys):
    print("  -", key)
PY

Repository: ambient-code/platform

Length of output: 422


Backend SDK option allowlist is missing manifest-defined keys.

The backend allowlist is missing 18 keys defined in the manifest: model, fallback_model, continue_conversation, session_id, settings, add_dirs, can_use_tool, cli_path, cwd, debug_stderr, fork_session, max_thinking_tokens, mcp_servers, permission_prompt_tool_name, resume, setting_sources, stderr, task_budget. Valid SDK options will be silently dropped at session creation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 68 - 204, The allowlist
defined by allowedSdkOptionKeys is missing 18 manifest-defined SDK option keys,
causing valid options to be dropped in filterSdkOptions; update the
allowedSdkOptionKeys map to include the keys listed in the comment (model,
fallback_model, continue_conversation, session_id, settings, add_dirs,
can_use_tool, cli_path, cwd, debug_stderr, fork_session, max_thinking_tokens,
mcp_servers, permission_prompt_tool_name, resume, setting_sources, stderr,
task_budget) and then add each key to the appropriate type maps
(sdkOptionStringKeys, sdkOptionNumericKeys, sdkOptionBoolKeys,
sdkOptionSliceKeys or leave as complex/object accepted by
validateSdkOptionValue) so validateSdkOptionValue enforces the correct types for
those new keys used by filterSdkOptions and session creation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design. The manifest is the superset of all ClaudeAgentOptions fields (39 keys). The backend allowlist is the safe subset (22 keys). The 18 "missing" keys are intentionally excluded:

  • Platform-internal (on runner denylist): cwd, resume, mcp_servers, setting_sources, continue_conversation, add_dirs, cli_path, stderr, env, api_key
  • Platform-controlled: fork_session, settings, permission_prompt_tool_name, session_id, debug_stderr
  • Callbacks: can_use_tool (programmatic only)
  • Set elsewhere: model, fallback_model (main session form model selector), max_thinking_tokens (via thinking config object), task_budget (new in 0.1.59, not yet surfaced)

The allowlist + denylist is the defense-in-depth security model. If a new field should be user-accessible, the drift detection will flag it in the SDK bump PR.

Comment on lines 226 to +227
if (advancedAgentOptions) {
request.agentOptions = agentOptionsForm.getValues();
request.sdkOptions = agentOptionsForm.getValues();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

sdkOptions never gets posted from this dialog under the new flag.

Line 227 only runs when advancedAgentOptions is true, but this dialog is still gated by useFlag("advanced-agent-options") at Line 89. The new flag added in components/manifests/base/core/flags.json is workspace-scoped advanced-sdk-options, and the dedicated component already uses useWorkspaceFlag(projectName, "advanced-sdk-options") in components/frontend/src/components/advanced-sdk-options.tsx. Result: this session-creation path stays dark and never sends sdkOptions when the workspace flag is enabled.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/create-session-dialog.tsx` around lines
226 - 227, The sdkOptions assignment is only executed when advancedAgentOptions
is true, but the workspace-scoped flag is "advanced-sdk-options" and is checked
elsewhere with useWorkspaceFlag, so request.sdkOptions never gets sent when the
workspace flag is enabled; update the condition around request.sdkOptions =
agentOptionsForm.getValues() (currently guarded by advancedAgentOptions) to also
check the workspace flag used by the advanced SDK component (e.g., call
useWorkspaceFlag(projectName, "advanced-sdk-options") or use the existing
workspaceFlag variable) so that the branch runs when either advancedAgentOptions
or the workspace "advanced-sdk-options" flag is enabled, ensuring sdkOptions is
posted during session creation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file (create-session-dialog.tsx) is dead code — nothing imports it. The active session creation path is NewSessionViewpage.tsx, which correctly wires sdkOptions through the AdvancedSdkOptions component with useWorkspaceFlag.

The type rename from agentOptionssdkOptions touched this file for consistency, but the dialog itself is unused. Deleting it entirely is a separate cleanup (it was removed in the original PR #1146 but still exists on main).

Comment thread specs/010-advanced-sdk-options/spec.md Outdated
Comment thread specs/010-advanced-sdk-options/tasks.md Outdated
jeremyeder added a commit that referenced this pull request Apr 16, 2026
## Summary

- Add `DropdownMenuSeparator` to the `+` dropdown, dividing context
actions (Add Repository, Upload File) from session options
- Add discovery dot on the `+` button — highlights new menu items using
`useLocalStorage` hook with version tracking
- Establishes the extensible pattern for per-session configuration:
boolean toggles as `DropdownMenuCheckboxItem`, form-heavy config as
`DropdownMenuItem` → `Dialog`

This PR adds no new session options — it creates the infrastructure that
PR #1326 (SDK Options) and PR #1328 (Project Intelligence) will build
on.

## Menu Pattern

```
[+] button (discovery dot when new items exist)
├── Add Repository          → existing modal
├── Upload File             → existing (PR #1282)
├── DropdownMenuSeparator
└── (future items: toggles inline, forms via modal)
```

## Test plan

- [x] Frontend test suite: 631 passed, 0 failed
- [x] Frontend build: 0 errors, 0 warnings
- [x] CodeRabbit CLI: 0 major/critical findings
- [x] Discovery dot: uses existing `useLocalStorage` hook (SSR-safe,
cross-tab sync)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

* **New Features**
* Added a visual separator in the session options dropdown menu for
improved organization.
* Introduced a discovery indicator dot on the "+" button to highlight
menu updates.
* The indicator dot automatically clears upon opening the dropdown menu.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Ambient Code Bot <bot@ambient-code.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ambient Code Bot and others added 15 commits April 16, 2026 16:36
Spec, plan, and task breakdown for re-implementing PR ambient-code#1146 —
exposing Claude Agent SDK options in session creation UI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix plan: note existing claude-agent-options/ form on main, derive
  allowlist from schema, add agentOptions→sdkOptions rename decision
- Rewrite tasks: merge TDD triplets into single tasks, remove dead code
  cleanup (create-session-dialog.tsx), remove per-phase lint duplication,
  fix allowlist to reference schema, add execution skill reference
- 22 tasks across 6 phases (down from 37 across 5)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…pt merge

Add _parse_sdk_options() to ClaudeBridge that reads SDK_OPTIONS from the
CR environment, filters denylisted keys (cwd, api_key, mcp_servers, etc.),
merges system_prompt under a Custom Instructions heading, and applies
remaining options to the adapter. Includes 30 tests covering valid/invalid
JSON, denylist enforcement, system prompt merge (string + dict formats),
and _ensure_adapter integration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add SDK options filtering and validation to the session creation handler.
User-provided sdkOptions are filtered against an allowlist of safe keys,
type-validated (string/numeric/bool/slice/complex), and serialized as
JSON into the SDK_OPTIONS environment variable on the CR spec. Unknown
keys are silently dropped; platform-internal keys (cwd, resume,
mcp_servers, api_key, etc.) are excluded.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… flag

Rename agentOptions to sdkOptions in session types. Create AdvancedSdkOptions
component using Shadcn Collapsible, gated by useWorkspaceFlag("advanced-sdk-options").
Wire into new-session-view with form value collection that filters defaults.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…flow

Remove standalone claude-sdk-options-drift.yml. Add drift detection step
to daily-sdk-update.yml so field changes are caught when the SDK version
bumps — same event, one PR instead of two.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…001)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on bump

- Delete daily-sdk-update.yml (targeted nonexistent claude-code-runner dir)
- Add SDK options drift check step to sdk-version-bump.yml
- Include manifest in PR when drift detected, append note to PR body
- Fix command injection: move ${{ }} expressions to env: blocks
- Fix shellcheck SC2016/SC2129 style warnings

One workflow, one PR, one human gate: version bump + field drift together.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… dict branch

Address CodeRabbit findings:
- drift check script: specify encoding="utf-8" on all file opens
- bridge.py: use suffix (with heading) instead of raw value in
  unknown dict shape branch for system_prompt merge

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Block cli_path (arbitrary binary injection) and env (environment variable
injection) from user-provided SDK_OPTIONS. Defense-in-depth.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- FR-005: add cli_path and env to denylist enumeration
- Tasks: replace standalone drift workflow with consolidated step

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tions

- Save button commits options, collapses form into compact badge summary
- Cancel with unsaved changes triggers dialog (save or discard)
- SDK options only sent on session create when explicitly saved
- Click saved summary to re-edit

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jeremyeder jeremyeder force-pushed the 010-advanced-sdk-options branch from 70c0c73 to 7f83b13 Compare April 16, 2026 20:40
- SDK Options now opens as a Dialog from the + dropdown menu item
- Menu item gated by advanced-sdk-options workspace flag
- AdvancedSdkOptions component simplified: no collapsible, no flag
  check (parent handles both), accepts onSave callback
- Separator only renders when flag-gated items are visible
- Updated tests for new component API

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/frontend/src/components/advanced-sdk-options.tsx`:
- Around line 22-33: The AdvancedSdkOptionsProps type declares projectName which
is never used in the AdvancedSdkOptions component; remove projectName from the
AdvancedSdkOptionsProps type and from the AdvancedSdkOptions parameter
destructuring (or alternatively use it inside the AdvancedSdkOptions function if
needed) so the prop list and component signature stay consistent (update
AdvancedSdkOptionsProps and the AdvancedSdkOptions function signature
accordingly).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c350ca14-9572-4f8d-b9ac-38b3fc24aeb7

📥 Commits

Reviewing files that changed from the base of the PR and between 7f83b13 and a19f253.

📒 Files selected for processing (4)
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/__tests__/new-session-view.test.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/new-session-view.tsx
  • components/frontend/src/components/__tests__/advanced-sdk-options.test.tsx
  • components/frontend/src/components/advanced-sdk-options.tsx
✅ Files skipped from review due to trivial changes (1)
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/tests/new-session-view.test.tsx

Comment on lines +22 to +33
type AdvancedSdkOptionsProps = {
projectName: string;
form: UseFormReturn<ClaudeAgentOptionsForm>;
disabled?: boolean;
onSave?: () => void;
};

export function AdvancedSdkOptions({
form,
disabled,
onSave,
}: AdvancedSdkOptionsProps) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unused projectName prop.

projectName is declared in props but never used in the component body. Remove it or use it.

 type AdvancedSdkOptionsProps = {
-  projectName: string;
   form: UseFormReturn<ClaudeAgentOptionsForm>;
   disabled?: boolean;
   onSave?: () => void;
 };

 export function AdvancedSdkOptions({
-  form,
+  form,
   disabled,
   onSave,
 }: AdvancedSdkOptionsProps) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type AdvancedSdkOptionsProps = {
projectName: string;
form: UseFormReturn<ClaudeAgentOptionsForm>;
disabled?: boolean;
onSave?: () => void;
};
export function AdvancedSdkOptions({
form,
disabled,
onSave,
}: AdvancedSdkOptionsProps) {
type AdvancedSdkOptionsProps = {
form: UseFormReturn<ClaudeAgentOptionsForm>;
disabled?: boolean;
onSave?: () => void;
};
export function AdvancedSdkOptions({
form,
disabled,
onSave,
}: AdvancedSdkOptionsProps) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/advanced-sdk-options.tsx` around lines 22
- 33, The AdvancedSdkOptionsProps type declares projectName which is never used
in the AdvancedSdkOptions component; remove projectName from the
AdvancedSdkOptionsProps type and from the AdvancedSdkOptions parameter
destructuring (or alternatively use it inside the AdvancedSdkOptions function if
needed) so the prop list and component signature stay consistent (update
AdvancedSdkOptionsProps and the AdvancedSdkOptions function signature
accordingly).

@jeremyeder jeremyeder merged commit e029650 into ambient-code:main Apr 16, 2026
34 of 35 checks passed
jeremyeder pushed a commit to jeremyeder/platform that referenced this pull request Apr 17, 2026
…nings

Analyzed 23 PRs merged since v0.2.0 and extracted 5 cross-cutting development
guidelines into CLAUDE.md Critical Conventions. Each prevents a specific class
of rework observed in recent PRs:

- Design for extensibility before adding items (PR ambient-code#1333ambient-code#1326)
- Verify contracts and references (PR ambient-code#1325, ambient-code#1329)
- CI/CD security (PR ambient-code#1310, ambient-code#1334, ambient-code#1331)
- Full-stack awareness (PR ambient-code#1282, ambient-code#1289)
- Separate configuration from code (PR ambient-code#1337)

Also renames "Critical Context" → "Critical Conventions" with a description
clarifying these are cross-cutting rules vs. component-specific ones.

Includes HTML triage dashboard used for HITL review of proposed guidelines.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jeremyeder pushed a commit to jeremyeder/platform that referenced this pull request Apr 17, 2026
…nings

Analyzed 23 PRs merged since v0.2.0 and extracted 5 cross-cutting development
guidelines into CLAUDE.md Critical Conventions. Each prevents a specific class
of rework observed in recent PRs:

- Design for extensibility before adding items (PR ambient-code#1333ambient-code#1326)
- Verify contracts and references (PR ambient-code#1325, ambient-code#1329)
- CI/CD security (PR ambient-code#1310, ambient-code#1334, ambient-code#1331)
- Full-stack awareness (PR ambient-code#1282, ambient-code#1289)
- Separate configuration from code (PR ambient-code#1337)

Also renames "Critical Context" → "Critical Conventions" with a description
clarifying these are cross-cutting rules vs. component-specific ones.

Includes HTML triage dashboard used for HITL review of proposed guidelines.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jeremyeder added a commit that referenced this pull request Apr 17, 2026
#1342)

## Summary

Analyzed 23 PRs merged since v0.2.0 and extracted 5 cross-cutting
development guidelines into CLAUDE.md Critical Conventions. Each
prevents a specific class of rework observed in recent PRs.

### Guidelines added to Critical Conventions

| Guideline | Source PRs | Rework Prevented |
|-----------|-----------|------------------|
| **Design for extensibility before adding items** | #1333#1326 | Menu
infrastructure built without pluggable pattern, downstream PR had to
rework separators |
| **Verify contracts and references** | #1325, #1329 | Hook used
nonexistent env var; script referenced old file path after restructuring
|
| **CI/CD security** | #1310, #1334, #1331 | `pull_request_target`
security hole; hardcoded tokens rotted silently |
| **Full-stack awareness** | #1282, #1289 | Existing S3 pipeline could
be reused; credential changes needed all consumers updated |
| **Separate configuration from code** | #1337 | Frontend hardcoded
localhost URL, broke kind/prod deployments |

### Also

- Renamed "Critical Context" → "Critical Conventions" with description
clarifying cross-cutting scope vs. component-specific conventions
- Included HTML triage dashboard (`docs/knowledge-finder-triage.html`)
used for HITL review

## Test plan

- [ ] Verify CLAUDE.md renders correctly and new conventions are clear
- [ ] Verify no duplication with existing conventions
- [ ] Verify HTML dashboard opens correctly in browser

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Updated development guidelines with revised conventions and
cross-cutting best practices for consistency and extensibility.
* Added new reference document for post-release item management and
workflow organization.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Ambient Code Bot <bot@ambient-code.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant